Skip to content

fix redirect uri validation to allow apps like: com.my.app:/ #243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MarvinG92
Copy link

This PR fixes a bug where valid mobile redirect URLs (e.g., custom schemes like com.my.app:/) were incorrectly rejected by the URL validation logic. Such redirect URIs are commonly used by mobile apps for OAuth flows and deep linking, and should be considered valid.

To clarify the issue and ensure it doesn’t regress in the future, I’ve also added a test case demonstrating the expected behavior with these kinds of URLs.

This is one way to solve the problem. Feel free to use a different solution if you prefer.

Here are some information about it: https://curity.io/resources/learn/oauth-for-mobile-apps-best-practices/

Changes:

Fixed the redirect URL validation logic to correctly handle custom mobile schemes

Added a unit test to verify acceptance of valid mobile redirect URIs

Test: ✅ Added test code to assert acceptance of com.my.app:/-style URLs

Test Code

<?php

function validUrl1(string $url) {
    $parsedUri = parse_url($url);

    return $parsedUri && isset($parsedUri['scheme']);
}

function validUrl2(string $url) {
    return preg_match('/^[a-zA-Z][a-zA-Z0-9+.-]*:(?:\/\/[^\/\s?#]+(?:\/[^\s?#]*)?|\/[^\s?#]*)?(?:\?[^\s#]*)?(?:#[^\s]*)?$/', $url) === 1;
}

echo 'Results with filter_var()'.PHP_EOL;
var_dump(filter_var('http://google.com', \FILTER_VALIDATE_URL));
var_dump(filter_var('http://google.com/test', \FILTER_VALIDATE_URL));
var_dump(filter_var('http://google.com/test?query=test', \FILTER_VALIDATE_URL));
var_dump(filter_var('invalid', \FILTER_VALIDATE_URL));
var_dump(filter_var('http://invalid url', \FILTER_VALIDATE_URL));
var_dump(filter_var('io.curity.client:/callback', \FILTER_VALIDATE_URL)); // false -> should return url
var_dump(filter_var('com.my.app:/', \FILTER_VALIDATE_URL)); // false -> should return url
var_dump(filter_var('myapp://callback#token=123', \FILTER_VALIDATE_URL));

echo PHP_EOL;

echo 'Results with parse_url()'.PHP_EOL;
var_dump(validUrl1('http://google.com'));
var_dump(validUrl1('http://google.com/test'));
var_dump(validUrl1('http://google.com/test?query=test'));
var_dump(validUrl1('invalid'));
var_dump(validUrl1('http://invalid url')); // true -> should be false because of whitespace
var_dump(validUrl1('io.curity.client:/callback'));
var_dump(validUrl1('com.my.app:/'));
var_dump(validUrl1('myapp://callback#token=123'));

echo PHP_EOL;

echo 'Results with preg_match()'.PHP_EOL;
var_dump(validUrl2('http://google.com'));
var_dump(validUrl2('http://google.com/test'));
var_dump(validUrl2('http://google.com/test?query=test'));
var_dump(validUrl2('invalid'));
var_dump(validUrl2('http://invalid url'));
var_dump(validUrl2('io.curity.client:/callback'));
var_dump(validUrl2('com.my.app:/'));
var_dump(validUrl2('myapp://callback#token=123'));

@chalasr
Copy link
Member

chalasr commented Jul 10, 2025

Would you mind covering this with a test to prevent regressions?

fix coding style in RedirectUri.php
@MarvinG92
Copy link
Author

Sure, I'd be happy to. I've added a test case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants